-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Keydb support #180
Keydb support #180
Conversation
REDIS_CONF, SMARTSIM_REDIS, SMARTSIM_REDIS_URL, SMARTSIM_REDIS_BRANCH need to be set for KeyDB
Codecov Report
@@ Coverage Diff @@
## develop #180 +/- ##
===========================================
+ Coverage 81.20% 81.66% +0.46%
===========================================
Files 57 57
Lines 2910 2973 +63
===========================================
+ Hits 2363 2428 +65
+ Misses 547 545 -2
|
Should we add tests for this code by using a new github actions job that does |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!!
Everything seems to work on my end, only change I'd request is adding smartsim/_core/bin/keydb-server
and smartsim/_core/bin/keydb-cli
to the .gitignore
I think that seems like a great idea! If not on this PR maybe in a new ticket? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice piece of work and it dovetails well with some of the things I'm trying to do. Main comment is to improve the safety of the code by removing the wildcards when removing files.
Looking forward to having this functionality in.
Review has been addressed & was given the green light via Slack
keydb flag for smart build added. Users can now use KeyDB in place of Redis more easily. [ committed by @EricGustin ] [ reviewed by @MattToast @ashao @Spartee ]
This PR adds the ability to use
KeyDB
in place ofRedis
as the database through a new--keydb
flag forsmart build
. This flexibility enables users to maximize their throughput if they desire.